Skip to content

test(rocm): ReshapeAndCache->PagedAttention composition at real dims (issue #41) - #497

Draft
VikashLoomba wants to merge 2 commits into
mudler:mainfrom
VikashLoomba:row/ROCM-ATTN-COMPOSE-TEST
Draft

test(rocm): ReshapeAndCache->PagedAttention composition at real dims (issue #41)#497
VikashLoomba wants to merge 2 commits into
mudler:mainfrom
VikashLoomba:row/ROCM-ATTN-COMPOSE-TEST

Conversation

@VikashLoomba

Copy link
Copy Markdown
Contributor

Row

BACKEND-ROCM — test-hardening only (one additive cross-device case). Issue #41.

What changed

Adds a cross-device case for the KV-cache composition the in-tree suite doesn't cover: the existing paged-attention case hand-builds a contiguous cache, but the real model path writes KV via ReshapeAndCache and reads it back via PagedAttention. This case is that composition at real Qwen3.5-0.8B dims (Dh=256, Hq=8, Hkv=2, block_size 16), a shuffled block table, and a non-sequential slot mapping — the layout a stride/scatter bug would live in and the contiguous case cannot see.

Surfaced by the #41 Qwen3.5-0.8B divergence investigation: with this composition passing, every piece of the ROCm attention path validates in isolation, which is what localizes the residual divergence to bf16-softmax accumulation rather than a kernel defect (full causal chain in this #41 comment).

Evidence (4× gfx1100, ROCm 7.14, Release)

  • New case: 7/7 vs the CPU oracle (runs on ROCm; the composition exercised end to end)
  • Full cross-device suite green
  • agent-preflight.sh --staged green; check-commit-trailers green

Speed claims

  • This PR makes NO speed claim.

Honest gaps

  • Test-only; no behavior change. The case validated the path as-is — it did not surface a defect (that was the point: it closes a coverage gap the divergence investigation needed ruled out).

@localai-org-maint-bot localai-org-maint-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new composition test does not build a self-consistent logical sequence. ReshapeAndCache writes tokens to (i*7+3)%64, while PagedAttention reads logical positions 0..19 through block table {3,1,2,0} (physical slots 48..63 and 16..19). Most read positions were never written, so CPU/device agreement can compare zeros and miss the intended scatter/layout defect. Please derive each token slot from its logical position and the shuffled block table (for example block_table[i/BS]*BS + i%BS), then add a mutation/guard proving a wrong physical-block mapping makes the test fail. Also remove the unused padded qstride: the tensor is declared contiguous, so those extra uploaded elements do not exercise a fused-view stride.

…udler#41)

The in-tree paged-attention case hand-builds a contiguous KV cache; the real
model path writes it with ReshapeAndCache and reads it back. This case is
that composition at real model dims (Dh=256, Hq=8, Hkv=2, block_size 16)
with a shuffled block table and non-sequential slot mapping — the layout a
stride/scatter bug would live in and the contiguous case cannot see.

Surfaced by the mudler#41 Qwen3.5-0.8B divergence investigation: every
compositional piece of the ROCm attention path now validates in isolation,
which is what localizes the residual divergence to bf16-softmax accumulation
rather than a kernel defect.

Evidence (4x gfx1100, ROCm 7.14, Release): the new case passes 7/7 vs the
CPU oracle; full cross-device suite green.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: pi:kimi-k3 [pi]
… table + an anti-vacuity guard -- the mudler#497 review rework

CHANGES_REQUESTED review (localai-org-maint-bot, 2026-08-14), both findings
accepted:

1. The first version scattered token i to slot (i*7+3)%64 while the attention
   read walked logical positions through block table {3,1,2,0} — the write set
   and the read set were disjoint by construction, so most positions compared
   unwritten zeros on both backends and a scatter/layout defect could not have
   failed the case. The slot mapping now derives from the logical position
   through the shuffled table (slots[i] = block_table[i/BS]*BS + i%BS), which
   is the engine's real mapping.
2. Anti-vacuity guard: a corrupted block table (the first two LOGICAL blocks
   swapped — both hold real tokens) must change the attention output; if the
   composition compared only unwritten slots it would not. (The first draft of
   the guard swapped two blocks OUTSIDE the logical range and was itself
   vacuous — the guard proved the guard; the committed version swaps the
   in-range mapping.) Also removed the unused padded qstride — the query
   tensor is declared contiguous, so the padding exercised nothing.

Gates (gfx1100, flock): test_backend_cross_device 20/20 with the corrected
composition.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: pi:kimi-k3 [pi]
@VikashLoomba
VikashLoomba force-pushed the row/ROCM-ATTN-COMPOSE-TEST branch from 6104ca1 to 8edebbd Compare August 14, 2026 09:07
@VikashLoomba

Copy link
Copy Markdown
Contributor Author

Both findings accepted and reworked (commit 8edebbd, rebased onto current main):

  1. Slot mapping derives from the block table now (slots[i] = block_table[i/BS]*BS + i%BS) — the engine's real mapping, so the reads actually hit the writes. You're right that the previous (i*7+3)%64 scatter was disjoint from the table and could compare zeros.
  2. Anti-vacuity guard added: a corrupted block table (first two logical blocks swapped — both hold real tokens) must change the attention output, proving the composition is load-bearing. (First draft of the guard swapped two blocks outside the logical range and was itself vacuous — the guard caught my guard; the committed version swaps in-range blocks.)
  3. The unused padded qstride is removed (the tensor is declared contiguous; the padding exercised nothing).

Gate: test_backend_cross_device 20/20 on gfx1100.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants